SK-3026 insert and detokenize added accordingt to doc - #382
Open
Conversation
The Codecov step uploaded target/site/jacoco/jacoco.xml, which was correct when this repo was a single Maven module. Since the common/v2/flowvault split the root pom is <packaging>pom</packaging> with no sources, so that file is never generated - codecov received nothing and reported no coverage for the PR. Verified locally: 'mvn -B package -f pom.xml' produces reports only at common/, v2/ and flowvault/ target/site/jacoco/jacoco.xml; the root path does not exist. Same class of issue as the 'Distribute test fixtures to modules' step already added on this branch - a repo-root path left over from the single-module layout.
Every individual_component path was rooted at src/main/java/..., which was correct when this was a single Maven module. Sources now live at <module>/src/main/java/..., so all 11 component globs matched zero files and each component reported no coverage data - which is the 'no coverage reports found' seen on the PR. The uploaded report itself was fine: the Codecov API shows state 'complete' for c1c5719 with 115 files at 91.75% (v2 55, common 30, flowvault 30), correctly path-resolved. Only the component matching was broken. Prefixed each glob with **/ so components aggregate a functional area across all modules, matching the previous semantics. Quoted because a bare leading * is a YAML alias. Verified against the real file list from the Codecov API: 0 components match zero files (was 11/11).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SK-3026: Align flowvault insert & detokenize with the FlowDB contract
Brings
flowvault's insert and detokenize request/response classes in line with the FlowDB Java SDK Contract, reduces the module to bulk-only, and adds request-fidelity coverage.52 files, +3,004 / −4,626 · 350 tests, 0 failures, 0 errors
Insert
Before | After -- | -- InsertRequest { table, upsert: List, upsertType, records } | { tableName, records: List, upsert: UpsertOptions } InsertRecord { table, data, upsert, upsertType } | new InsertRequestRecord { tableName, data, tokens, upsert } — adds BYOT tokens — | new UpsertOptions { String updateType, List uniqueColumns } InsertResponse { insertedFields, errors } | { records: List } — failures inline — | new InsertResponseRecord { tableName, skyflowId, fields, hashedData, httpCode, error } BulkInsertRequest (standalone) | extends InsertRequest, no fields of its own BulkInsertRecord | deleted → new BulkInsertRequestRecord extends InsertRequestRecord BulkInsertResponse { summary, success, errors } | { records: List, summary: BulkSummary } Success, Summary, Token | deleted — bulk-insert-only, replaced aboveTests
+55 request-fidelity tests asserting user input reaches the wire.
assertSameondata/tokens/uniqueColumnsproves pass-through rather than reserialization. Batching verified end-to-end withArgumentCaptorover real dispatched batches: order preserved,vaultIdandtokenGroupRedactionson every batch, responseindexmatching original input position, and interceptor headers reaching each batch with a distinctRequestContext.Non-ASCII values, spaced keys, and non-string
data(Integer,Double,Boolean, nestedMap/List) all verified to keep type and identity.📄
docs/manual-test-scenarios.md— sections A–D cover flowvault (bulk insert, detokenize, tokenize/deleteTokens, headers & config).Dependency
Depends on the
commonbase classes becoming empty markers (BaseInsertRequest,BaseInsertResponse,BaseDetokenizeData) — flowvault extends them for future use. That change also touchesv2and breaks its japicmp gate; see the companion change for the baseline regeneration.SK-3026: Align flowvault insert & detokenize with the FlowDB contract Brings flowvault's insert and detokenize request/response classes in line with the [FlowDB Java SDK Contract](https://skyflow.atlassian.net/wiki/spaces/SDK1/pages/3039166470/FlowDB+Java+SDK+Contract), reduces the module to bulk-only, and adds request-fidelity coverage.
52 files, +3,004 / −4,626 · 350 tests, 0 failures, 0 errors
Insert
Before After
InsertRequest { table, upsert: List, upsertType, records } { tableName, records: List, upsert: UpsertOptions }
InsertRecord { table, data, upsert, upsertType } new InsertRequestRecord { tableName, data, tokens, upsert } — adds BYOT tokens
— new UpsertOptions { String updateType, List uniqueColumns }
InsertResponse { insertedFields, errors } { records: List } — failures inline
— new InsertResponseRecord { tableName, skyflowId, fields, hashedData, httpCode, error }
BulkInsertRequest (standalone) extends InsertRequest, no fields of its own
BulkInsertRecord deleted → new BulkInsertRequestRecord extends InsertRequestRecord
BulkInsertResponse { summary, success, errors } { records: List, summary: BulkSummary }
Success, Summary, Token deleted — bulk-insert-only, replaced above
Detokenize
Before After
DetokenizeRequest { detokenizeData: List } { tokens: List, tokenGroupRedactions }
DetokenizeResponse { detokenizedFields, errors } { records: List }
DetokenizeRecordResponse deleted → new DetokenizeResponseRecord { token, value, tokenGroupName, metadata, httpCode, error }
BulkDetokenizeRequest (standalone) extends DetokenizeRequest
BulkDetokenizeResponse { summary, success, errors } { records: List, summary }
DetokenizeData, DetokenizeResponseObject, BulkTokenGroupRedactions deleted
The success/errors split is gone on both — failures are inline with per-record httpCode/error. Serialization moved from excludeFieldsWithoutExposeAnnotation() to serializeNulls() with transient internals, matching the doc's sample JSON.
Bulk-only
All unary methods removed from VaultController; the 8 bulk methods remain. Also removed: 16 orphaned classes (Get*, Query*, Tokenize*, DeleteTokens* request/response), 16 dead Utils/Validations methods, 36 unused imports, and implements IVaultController.
InsertResponse and DetokenizeResponse are retained as published API despite having no caller, since the contract specifies them.
Validation
Table name and upsert at exactly one level. tableName at request level or on every record, never both. upsert is optional but must match the table's level; it need not appear on every record.
updateType validated against FlowEnumUpdateType. Previously an unrecognized value ("MERGE", "update ") was silently dropped and the upsert ran with server defaults.
10,000-record cap enforced on all four bulk ops, using error constants that already existed unreferenced. Flips a prior test that asserted no cap.
UpsertType deleted — a hand-written duplicate of the generated FlowEnumUpdateType. Validator and mapper now read the same enum, so they can't drift.
Bug fixes
Fix Impact
NPE in getRecordsToRetry() / getTokensToRetry() A per-batch response with any 5xx record threw NPE
Blank tableName reaching the wire Validator treated " " as absent, mapper as present
Dead upsert wiring in insertBatchFutures Always-empty read; insertBatch always passed null
.exceptionally() side-channel Bulk insert/detokenize collected errors out-of-band and returned null
Tests
+55 request-fidelity tests asserting user input reaches the wire. assertSame on data/tokens/uniqueColumns proves pass-through rather than reserialization. Batching verified end-to-end with ArgumentCaptor over real dispatched batches: order preserved, vaultId and tokenGroupRedactions on every batch, response index matching original input position, and interceptor headers reaching each batch with a distinct RequestContext.
Non-ASCII values, spaced keys, and non-string data (Integer, Double, Boolean, nested Map/List) all verified to keep type and identity.
📄 docs/manual-test-scenarios.md — sections A–D cover flowvault (bulk insert, detokenize, tokenize/deleteTokens, headers & config).
Dependency
Depends on the common base classes becoming empty markers (BaseInsertRequest, BaseInsertResponse, BaseDetokenizeData) — flowvault extends them for future use. That change also touches v2 and breaks its japicmp gate; see the companion change for the baseline regeneration.